Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't include objc_library generated module maps in packaged frameworks #180

Merged
merged 3 commits into from
Jan 5, 2021

Conversation

segiddins
Copy link
Member

Fixes #179, by no longer leaking an incorrect module map

@@ -172,6 +172,10 @@ def _apple_framework_packaging_impl(ctx):

# collect modulemaps
for modulemap in dep[apple_common.Objc].direct_module_maps:
if modulemap.owner == dep.label:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make sure I understand this correctly, for direct module maps of a given obj-c dependency, if the module map is generated by the dep itself, including it will cause redefinition error, so skip them at this point is the way to fix this error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Contributor

@gyfelton gyfelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@tinder-maxwellelliott
Copy link
Collaborator

Could we expand the test setup to cover more complex situations i.e. Unit Tests with bridging headers that import headers from the tested target? I have made the changes in this commit 1bd0996

@tinder-maxwellelliott
Copy link
Collaborator

Actually I can break this when using a bridging header with unit tests, see changes here #181

@segiddins segiddins force-pushed the segiddins/no-generated-objc-library-module-maps branch from 6913263 to 9f3fa62 Compare January 5, 2021 22:24
@tinder-maxwellelliott
Copy link
Collaborator

Looping back, my import style in my bridging header was breaking this. It would be great if we could also merge in the test updates here #181

@segiddins segiddins merged commit 20d809d into master Jan 5, 2021
@segiddins segiddins deleted the segiddins/no-generated-objc-library-module-maps branch January 5, 2021 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Seeing symbol redefinition issues when using a bridging header
6 participants